-
Notifications
You must be signed in to change notification settings - Fork 69
Socket implementation #127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
efcec20 to
14b4026
Compare
hexagonal-sun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, some minor comments. Hopefully they'll fix things up nicely. One final point.. perhaps it's worth putting all this code under net, rather than socket? That seems to be the idiomatic name that most kernels use for this kind of code.
|
|
||
| impl TcpSocket { | ||
| pub fn new() -> Self { | ||
| let rx_buffer = SocketBuffer::new(vec![0; 4096]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than using the heap, maybe dedicate a ClaimedPage here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SocketBuffer needs something that can be converted into a ManagedSlice, so I have to give it an owned vec. Trying to give it a borrowed slice runs into issues because the slice would be given a lifetime of 'static
| handle, | ||
| local_endpoint: SpinLock::new(None), | ||
| backlogs: SpinLock::new(Vec::new()), | ||
| num_backlogs: AtomicUsize::new(0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I quite get why we track this separately? Couldn't we call backlogs.len()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to reduce calls into the SpinLock for performance.
| } | ||
|
|
||
| #[async_trait] | ||
| impl SocketOps for UnixSocket { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I almost wonder whether we make a bit of an architectural change here and consider splitting the trait into two types.. a listening socket and a connected socket? Feels like this might make things easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then the registry would only contain listening sockets.
| if *self.rd_shutdown.lock_save_irq() { | ||
| return Ok(0); | ||
| } | ||
| self.inbox.copy_to_user(buf, count).await |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once thing to note, SeqPacket and Datagram sockets will not preserve message boundaries with the kbuf, it's just a stream of bytes; a perfect fit for Stream sockets.
We need to think about how we can preserve message boundaries.
d2a23d8 to
d17077d
Compare
Implements socket syscalls (i.e.
socket,accept,connect,bind, etc.) and unix sockets. Also stubs out part of a TCP socket implementation.